Change .call() for modern spread call #4504
Conversation
| if (originalConsoleLevel) { | ||
| Function.prototype.apply.call(originalConsoleLevel, global.console, args); | ||
| if (originalConsoleLevel && 'originalConsoleLevel' in global.console) { | ||
| global.console[originalConsoleLevel](...args); |
There was a problem hiding this comment.
Thanks for the contribution! Unfortunately, we can't accept it, because it both causes a build error and breaks the functionality of the code.
Remember that originalConsoleLevel is a function, which is why the build throws this error:
Further, originalConsoleLevel is not a property of global.console. I suspect you're thinking of level instead, which is the name of the method in question ("warn", "error", and so on), which IS a property of global.console. That said, changing originalConsoleLevel in your second line to level also doesn't fix it, because now global.console[level] is the wrapped function, not the original.
All of that said:
-
I can understand how you got confused, because
originalConsoleLevelsounds like it should be a string. I'll give it a better name. -
You're not wrong that it can be simplified: it can be changed to
originalConsoleLevel.call(global.console, args);, which I will also do.
Cheers!
There was a problem hiding this comment.
Thanks for the explanation, I didn't test or check the types yet (just made this in Github editor as an example). Thanks for taking it over and applying this change, this will save some errors in our sentry :)
…e` (#4505) In responding to #4504, I noticed two things which could be improved about the code in question: 1) The author of that PR was misled by the admittedly confusing name of the original method we're wrapping. 2) We've been calling that method in a more complex way than necessary. This fixes both of those problems.
The current logic causes the following warning:
There's no need to do a
.call()here we can just use argument spread.